Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enable os5 for MOTE_L152RC #6250

Closed
wants to merge 6 commits into from
Closed

enable os5 for MOTE_L152RC #6250

wants to merge 6 commits into from

Conversation

dudmuck
Copy link
Contributor

@dudmuck dudmuck commented Mar 1, 2018

Description

Enable mbed-os5 for target MOTE_L152RC.
Tested using: mbed test -t GCC_ARM -m MOTE_L152RC -n mbed-os-*
Removed "default_lib": "small" for this target because c_string float printf was failing.

Pull request type

[x] Fix
[ ] Refactor
[ ] New target
[ ] Feature
[ ] Breaking change

@cmonr
Copy link
Contributor

cmonr commented Mar 2, 2018

@dudmuck Thanks for the PR. Do you think you could post the results for the test here? A text file would be fine.

Also, have you tested against the ARM and IAR compilers?

@cmonr
Copy link
Contributor

cmonr commented Mar 2, 2018

@Patater Mis-clicked when assigning reviewers. Feel free to dismiss yourself or ignore this PR.

@cmonr
Copy link
Contributor

cmonr commented Mar 2, 2018

@dudmuck Making sure, is this targeted for the NaMote-72 (https://os.mbed.com/platforms/NAMote-72/)? I didn't seem to find the MOTE_L152RC anywhere, and think I'm missing something.

@dudmuck
Copy link
Contributor Author

dudmuck commented Mar 2, 2018

Yes, TARGET_MOTE_L152 is NAMote-72 platform. I have tested using ARM GCC, but i dont have armcc for cortex-m3 or IAR ARM.
I tested using -n mbed-os-*
All was passing except c_string float printf because default_lib was small.

@0xc0170 0xc0170 removed the request for review from Patater March 5, 2018 09:05
@cmonr
Copy link
Contributor

cmonr commented Mar 6, 2018

@hasnainvirk Thoughts?

@0xc0170
Copy link
Contributor

0xc0170 commented Mar 12, 2018

All was passing except c_string float printf because default_lib was small.

Should pass. As this has uARM that is not thread safe and it's very small target, I do not think enabling mbed OS 5 for this platform makes sense.

@0xc0170
Copy link
Contributor

0xc0170 commented Mar 13, 2018

Should pass. As this has uARM that is not thread safe and it's very small target, I do not think enabling mbed OS 5 for this platform makes sense.

@ARMmbed/team-st-mcd What do you think? I checked the linker files, this target has 256k flash and 32k RAM.

@0Grit
Copy link

0Grit commented Mar 13, 2018

@0xc0170
I've been meaning to attempt a similar pull request for some time, as we would like to use OS5 with this board.

What are the general hardware requirements for OS5?

Not that it would help those who already have the board but;
I have some influence in regards to this board's design, and might be able to push for a replacement platform or changes if they make sense.

@bcostm
Copy link
Contributor

bcostm commented Mar 14, 2018

What do you think? I checked the linker files, this target has 256k flash and 32k RAM.

Well, on our side we have enabled OS5 only on targets with more than 64KB Flash AND more than 16KB RAM. So, OS5 should be ok for this board...

@0Grit
Copy link

0Grit commented Mar 14, 2018

@cmonr @0xc0170 What more is needed to make this happen?

@janjongboom Any interest in OS5 support for MOTE_L152RC

@0xc0170
Copy link
Contributor

0xc0170 commented Mar 15, 2018

All was passing except c_string float printf because default_lib was small.

This should be fixed. First, why this target has uARM if it has 256k flash and 32k RAM. If we remove this from the target, will all test pass?

@dudmuck Can you test please?

@loverdeg-ep Thanks for the feedback, this should be fine to have it enabled for Mbed OS 5. needs to pass all tests.

@janjongboom
Copy link
Contributor

janjongboom commented Mar 15, 2018

@loverdeg-ep Yes, very happy with this, so we can run the new LoRaWAN stack on this target too. As @0xc0170 said this target should be able to run normal default lib. xDot has same RAM and ROM and runs everything fine, with >50% space left when running LoRaWAN stack and RTOS.

@cmonr
Copy link
Contributor

cmonr commented Mar 15, 2018

Will start CI when able. Still waiting on feedback from @hasnainvirk and @ashok-rao, but I think @bcostm's and @janjongboom's feedback indicate this is good to go.

@cmonr
Copy link
Contributor

cmonr commented Mar 15, 2018

Also still waiting on this.

All was passing except c_string float printf because default_lib was small.

This should be fixed. First, why this target has uARM if it has 256k flash and 32k RAM. If we remove this from the target, will all test pass?

@dudmuck Can you test please?

@0xc0170
Copy link
Contributor

0xc0170 commented Apr 4, 2018

Closing this due to inactivity. Please reopen with an update. It would be nice addition based on the feedback here

@0xc0170 0xc0170 closed this Apr 4, 2018
@hasnainvirk
Copy link
Contributor

This platform could potentially run LoRaWAN stack on it. @dudmuck could you please investigate why you are having troubles with default lib ? It should be alright in my opinion. You may add ARM and GCC_ARM in supported toolchains.

@maclobdell
Copy link
Contributor

Reopening due to new activity/fixes/test results.
@dudmuck - please add new information that you have available.

@maclobdell maclobdell reopened this Jun 6, 2018
@dudmuck
Copy link
Contributor Author

dudmuck commented Jun 6, 2018

test results attached
MOTE_IAR.LOG
MOTE_GCC.LOG
MOTE_ARM.LOG

@cmonr
Copy link
Contributor

cmonr commented Jun 6, 2018

@dudmuck @maclobdell @hasnainvirk thanks for getting the ball moving again with this.

However, before we can progress, the second commit needs to be removed (Merge remote-tracking branch 'upstream/master' into mote_L152RC_add_os5). Our branching model doesn't support merges from master into PRs. Instead, you need to do a rebase, using upstream/master as the base branch.

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 7, 2018

6 commits but changes are only in targets.json file (few lines), squash to one commit (also merge commit will disappear).

I reviewed the tests, nvstore failed. Is this related to this fix ? #7127 . If not , please provide details for the failure

@0xc0170
Copy link
Contributor

0xc0170 commented Jun 15, 2018

This should actually resolve #4394

@cmonr
Copy link
Contributor

cmonr commented Jun 21, 2018

@dudmuck Any progress?

@cmonr
Copy link
Contributor

cmonr commented Jun 25, 2018

Closing due to no response to comments for nearly three weeks.
Feel free to reopen once an update is available.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants